compression: add zstd compressor and decompressor by rainingmaster · Pull Request #20434 · envoyproxy/envoy

rainingmaster · 2022-03-21T13:32:46Z

Hi! We are from the Service Mesh team at ByteDance and are excited to have the opportunity to contribute to Envoy. Zstd is Facebook's open source compression algorithm, and it is getting more and more attention. In ByteDance's Service Mesh, zstd is also being used more and more. Therefore, we hope envoy can integrate this algorithm and let more people use it.

Commit Message: compression: add zstd compressor and decompressor
Additional Description: Like #12998, add new zstd(alson name as Zstandard) compression extensions in addition to gzip, brotli.
Risk Level: Low, add context for createCompressorFactoryFromProtoTyped in exist code.
Testing: uni tests, manual tests with curl.
Docs Changes: updated docs for compression and decompression HTTP filters to refer the new available encoder/decoder.
Release Notes: updated current.rst

The PR adds a new dependency on https://github.com/facebook/zstd. Here's the current criteria answers:

Criteria	Answer
Cloud Native Computing Foundation (CNCF) approved license	BSD
Dependencies must not substantially increase the binary size unless they are optional	origin: 326M, new: 330M
No duplication of existing dependencies	no other dep provides zstd
Hosted on a git repository and the archive fetch must directly reference this repository.	https://github.com/facebook/zstd
CVE history appears reasonable, no pathological CVE arcs.	so far 3 CVEs related to zstd have been registered: https://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=zstd
Code review (ideally PRs) before merge.	PRs are reviewed before merge
Security vulnerability process exists, with contact details and reporting/disclosure process.	https://github.com/facebook/zstd/blob/dev/CONTRIBUTING.md#issues
> 1 contributor responsible for a non-trivial number of commits.	242 contributors
Tests run in CI.	CI set up with Travis CI
High test coverage (also static/dynamic analysis, fuzzing).	Fuzzers are run in CI
Envoy can obtain advanced notification of vulnerabilities or of security releases.	zstd is registered in CPE
Do other significant projects have shared fate by using this dependency?	The Linux kernel has included Zstandard since November 2017 (version 4.14) as a compression method for the btrfs and squashfs filesystems
Releases (with release notes).	https://github.com/facebook/zstd/releases
Commits/releases in last 90 days.	last commit 2 days ago

repokitteh-read-only · 2022-03-21T13:32:58Z

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @markdroth
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).
CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).
envoyproxy/dependency-shepherds assignee is @mattklein123

🐱

Caused by: #20434 was opened by rainingmaster.

see: more, trace.

mattklein123 · 2022-03-21T15:28:38Z

I'm assigning this over to @rojkov as compression expert.

Also, can you please provide some context and rational for this feature? In general for something this large we require sponsorship and an issue tracking the feature addition. Without sponshorship this will need to go into contrib. Thank you.

rainingmaster · 2022-03-21T15:56:53Z

I'm assigning this over to @rojkov as compression expert.

Also, can you please provide some context and rational for this feature? In general for something this large we require sponsorship and an issue tracking the feature addition. Without sponshorship this will need to go into contrib. Thank you.

Thanks for your help and reminder! I will provide the required information after fixing the CI error.

moderation · 2022-03-21T16:01:48Z

scorecard --repo=https://github.com/facebook/zstd/

RESULTS
-------
Aggregate score: 6.6 / 10

Check scores:
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------|
|  SCORE  |          NAME          |             REASON             |                             DOCUMENTATION/REMEDIATION                             |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------|
| 10 / 10 | Binary-Artifacts       | no binaries found in the repo  | https://github.com/ossf/scorecard/blob/main/docs/checks.md#binary-artifacts       |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------|
| 1 / 10  | Branch-Protection      | branch protection is not       | https://github.com/ossf/scorecard/blob/main/docs/checks.md#branch-protection      |
|         |                        | maximal on development and all |                                                                                   |
|         |                        | release branches               |                                                                                   |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------|
| 10 / 10 | CI-Tests               | 30 out of 30 merged PRs        | https://github.com/ossf/scorecard/blob/main/docs/checks.md#ci-tests               |
|         |                        | checked by a CI test -- score  |                                                                                   |
|         |                        | normalized to 10               |                                                                                   |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------|
| 0 / 10  | CII-Best-Practices     | no badge detected              | https://github.com/ossf/scorecard/blob/main/docs/checks.md#cii-best-practices     |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------|
| 10 / 10 | Code-Review            | all last 30 commits are        | https://github.com/ossf/scorecard/blob/main/docs/checks.md#code-review            |
|         |                        | reviewed through GitHub        |                                                                                   |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------|
| 10 / 10 | Contributors           | 15 different companies found   | https://github.com/ossf/scorecard/blob/main/docs/checks.md#contributors           |
|         |                        | -- score normalized to 10      |                                                                                   |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------|
| 10 / 10 | Dangerous-Workflow     | no dangerous workflow patterns | https://github.com/ossf/scorecard/blob/main/docs/checks.md#dangerous-workflow     |
|         |                        | detected                       |                                                                                   |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------|
| 0 / 10  | Dependency-Update-Tool | no update tool detected        | https://github.com/ossf/scorecard/blob/main/docs/checks.md#dependency-update-tool |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------|
| 10 / 10 | Fuzzing                | project is fuzzed in OSS-Fuzz  | https://github.com/ossf/scorecard/blob/main/docs/checks.md#fuzzing                |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------|
| 10 / 10 | License                | license file detected          | https://github.com/ossf/scorecard/blob/main/docs/checks.md#license                |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------|
| 10 / 10 | Maintained             | 30 commit(s) out of 30 and 0   | https://github.com/ossf/scorecard/blob/main/docs/checks.md#maintained             |
|         |                        | issue activity out of 30 found |                                                                                   |
|         |                        | in the last 90 days -- score   |                                                                                   |
|         |                        | normalized to 10               |                                                                                   |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------|
| ?       | Packaging              | no published package detected  | https://github.com/ossf/scorecard/blob/main/docs/checks.md#packaging              |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------|
| 6 / 10  | Pinned-Dependencies    | dependency not pinned by hash  | https://github.com/ossf/scorecard/blob/main/docs/checks.md#pinned-dependencies    |
|         |                        | detected -- score normalized   |                                                                                   |
|         |                        | to 6                           |                                                                                   |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------|
| 0 / 10  | SAST                   | SAST tool is not run on all    | https://github.com/ossf/scorecard/blob/main/docs/checks.md#sast                   |
|         |                        | commits -- score normalized to |                                                                                   |
|         |                        | 0                              |                                                                                   |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------|
| 10 / 10 | Security-Policy        | security policy file detected  | https://github.com/ossf/scorecard/blob/main/docs/checks.md#security-policy        |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------|
| 6 / 10  | Signed-Releases        | 3 out of 5 artifacts are       | https://github.com/ossf/scorecard/blob/main/docs/checks.md#signed-releases        |
|         |                        | signed -- score normalized to  |                                                                                   |
|         |                        | 6                              |                                                                                   |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------|
| 0 / 10  | Token-Permissions      | non read-only tokens detected  | https://github.com/ossf/scorecard/blob/main/docs/checks.md#token-permissions      |
|         |                        | in GitHub workflows            |                                                                                   |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------|
| 10 / 10 | Vulnerabilities        | no vulnerabilities detected    | https://github.com/ossf/scorecard/blob/main/docs/checks.md#vulnerabilities        |
|---------|------------------------|--------------------------------|-----------------------------------------------------------------------------------|

rojkov · 2022-03-22T09:01:58Z

You can turn this PR into a draft until it's ready for review. After the PR is ready please avoid force-pushing.

rainingmaster · 2022-03-22T09:52:50Z

You can turn this PR into a draft until it's ready for review. After the PR is ready please avoid force-pushing.

Thanks for your remind, done

rojkov · 2022-03-24T08:48:10Z

I've discussed the feature with my colleagues. Intel needs it as a benchmark for the upcoming zSTD acceleration in QuickAssist HW and is willing to support it together with ByteDance.

@rainingmaster Could you please file an issue for the feature, so I could state my sponsorship?

repokitteh-read-only · 2022-04-07T14:32:58Z

Retrying Azure Pipelines:
Check envoy-presubmit isn't fully completed, but will still attempt retrying.
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #20434 (comment) was created by @rainingmaster.

see: more, trace.

rainingmaster · 2022-04-07T15:43:15Z

/retest

repokitteh-read-only · 2022-04-07T15:43:19Z

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #20434 (comment) was created by @rainingmaster.

see: more, trace.

markdroth · 2022-04-07T15:53:18Z

/lgtm api

mattklein123

Thanks LGTM with tiny comment.

/wait

api/envoy/extensions/compression/zstd/compressor/v3/zstd.proto

rainingmaster · 2022-04-08T02:02:50Z

/retest

repokitteh-read-only · 2022-04-08T02:02:53Z

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #20434 (comment) was created by @rainingmaster.

see: more, trace.

rainingmaster · 2022-04-08T06:24:13Z

/retest

repokitteh-read-only · 2022-04-08T06:24:16Z

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #20434 (comment) was created by @rainingmaster.

see: more, trace.

Signed-off-by: rainingmaster <jinhua.tan@bytedance.com>

rainingmaster · 2022-04-08T12:36:52Z

/retest

repokitteh-read-only · 2022-04-08T12:36:55Z

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #20434 (comment) was created by @rainingmaster.

see: more, trace.

Signed-off-by: rainingmaster <jinhua.tan@bytedance.com>

rainingmaster · 2022-04-11T13:09:38Z

Hi, @mattklein123 . I think we are very close now, could you help me review again? Thanks!

mattklein123

Thanks!

Signed-off-by: rainingmaster <jinhua.tan@bytedance.com> Signed-off-by: Andre Vehreschild <vehre@x41-dsec.de>

Signed-off-by: rainingmaster <jinhua.tan@bytedance.com>

rainingmaster requested review from dio and rojkov as code owners March 21, 2022 13:32

repokitteh-read-only bot added api deps Approval required for changes to Envoy's external dependencies labels Mar 21, 2022

repokitteh-read-only bot assigned markdroth and mattklein123 Mar 21, 2022

rainingmaster mentioned this pull request Mar 21, 2022

compression: add zstd compressor and decompressor rainingmaster/envoy#1

Closed

rainingmaster force-pushed the zstd branch from a67b0a2 to 3001795 Compare March 21, 2022 13:34

rainingmaster changed the title ~~compression: add zstd compressor and decompressor~~ WIP: compression: add zstd compressor and decompressor Mar 21, 2022

rainingmaster force-pushed the zstd branch from 3001795 to cf45467 Compare March 21, 2022 15:21

mattklein123 assigned rojkov and unassigned mattklein123 Mar 21, 2022

rainingmaster force-pushed the zstd branch 2 times, most recently from aec29c2 to 23f2966 Compare March 21, 2022 15:58

rainingmaster force-pushed the zstd branch 2 times, most recently from 0703485 to 44e0edf Compare March 22, 2022 02:38

rainingmaster requested a review from lizan as a code owner March 22, 2022 02:38

rainingmaster force-pushed the zstd branch 2 times, most recently from 17c6902 to f845d56 Compare March 22, 2022 08:34

rainingmaster marked this pull request as draft March 22, 2022 09:51

rainingmaster changed the title ~~WIP: compression: add zstd compressor and decompressor~~ compression: add zstd compressor and decompressor Mar 22, 2022

rainingmaster force-pushed the zstd branch 2 times, most recently from caaa029 to 6246571 Compare March 22, 2022 12:31

repokitteh-read-only bot removed the api label Apr 7, 2022

mattklein123 requested changes Apr 7, 2022

View reviewed changes

api/envoy/extensions/compression/zstd/compressor/v3/zstd.proto Outdated Show resolved Hide resolved

repokitteh-read-only bot added the waiting label Apr 7, 2022

fix example test, follow 20397

73bdb15

Signed-off-by: rainingmaster <jinhua.tan@bytedance.com>

repokitteh-read-only bot removed the waiting label Apr 8, 2022

repokitteh-read-only bot added the waiting label Apr 8, 2022

fix link in docs

2689679

Signed-off-by: rainingmaster <jinhua.tan@bytedance.com>

repokitteh-read-only bot added api and removed waiting labels Apr 9, 2022

rainingmaster requested a review from mattklein123 April 9, 2022 12:30

mattklein123 approved these changes Apr 11, 2022

View reviewed changes

repokitteh-read-only bot removed api deps Approval required for changes to Envoy's external dependencies labels Apr 11, 2022

mattklein123 merged commit 8655952 into envoyproxy:main Apr 11, 2022

vehre-x41 pushed a commit to vehre-x41/envoy that referenced this pull request Apr 19, 2022

compression: add zstd compressor and decompressor (envoyproxy#20434)

9d82cbc

Signed-off-by: rainingmaster <jinhua.tan@bytedance.com> Signed-off-by: Andre Vehreschild <vehre@x41-dsec.de>

mkauf mentioned this pull request Apr 21, 2022

Failure when building the zstd library #20923

Closed

ravenblackx pushed a commit to ravenblackx/envoy that referenced this pull request Jun 8, 2022

compression: add zstd compressor and decompressor (envoyproxy#20434)

778f6a5

Signed-off-by: rainingmaster <jinhua.tan@bytedance.com>

KBaichoo mentioned this pull request Oct 1, 2025

compressor: expose compressor_library for per-route overrides #41177

Merged

Conversation

rainingmaster commented Mar 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

repokitteh-read-only bot commented Mar 21, 2022

Uh oh!

mattklein123 commented Mar 21, 2022

Uh oh!

rainingmaster commented Mar 21, 2022

Uh oh!

moderation commented Mar 21, 2022

Uh oh!

rojkov commented Mar 22, 2022

Uh oh!

rainingmaster commented Mar 22, 2022

Uh oh!

rojkov commented Mar 24, 2022

Uh oh!

repokitteh-read-only bot commented Apr 7, 2022

Uh oh!

rainingmaster commented Apr 7, 2022

Uh oh!

repokitteh-read-only bot commented Apr 7, 2022

Uh oh!

markdroth commented Apr 7, 2022

Uh oh!

mattklein123 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rainingmaster commented Apr 8, 2022

Uh oh!

repokitteh-read-only bot commented Apr 8, 2022

Uh oh!

rainingmaster commented Apr 8, 2022

Uh oh!

repokitteh-read-only bot commented Apr 8, 2022

Uh oh!

rainingmaster commented Apr 8, 2022

Uh oh!

repokitteh-read-only bot commented Apr 8, 2022

Uh oh!

rainingmaster commented Apr 11, 2022

Uh oh!

mattklein123 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

rainingmaster commented Mar 21, 2022 •

edited

Loading